Skip to content

fix(mcp): reject stale AST coordinates after source changes - #2216

Open
lorenzozanee wants to merge 1 commit into
DeusData:mainfrom
lorenzozanee:restore/pr-2105
Open

lorenzozanee wants to merge 1 commit into
DeusData:mainfrom
lorenzozanee:restore/pr-2105

Conversation

@lorenzozanee

@lorenzozanee lorenzozanee commented Sep 15, 2026

Copy link
Copy Markdown
Contributor

Fixes #1750. Source edits can invalidate AST line ranges, so this update uses the canonical file metadata on Windows before exposing stored coordinates, rejects stale snippet reads, and preserves raw search matches without stale AST attribution.

Supersedes #2105(原PR分支已删,按原提交重建)

@github-actions

Copy link
Copy Markdown

Thanks for opening this — it has been seen, and it is queued.

This note is automated, but it is not a brush-off: it exists so you know where your PR stands instead of having to guess from silence.

Current review status: working through a backlog. 0.9.1-rc.1 is out, so the release freeze that held reviews is over — but it left a large queue of open pull requests behind it, and we are reading through them oldest-first. The background is in discussion #1144.

What that means for this PR, concretely:

  • It will not be closed for inactivity. No stale bot touches pull requests here.
  • It may still sit a while before a human reads it. That is on us, not on you.
  • Older PRs are read first, so a recent one is not being skipped — it is behind a queue.

Things that will genuinely speed it up whenever review does happen:

  • Keep it rebased on main — the tree is moving quickly right now, and a conflicting branch cannot be reviewed as the diff you intended.
  • Get CI green, or say which failures you believe are pre-existing.
  • Keep the change to one claim. Bundled features and refactors get split before they get merged, which costs you a round trip.
  • Every commit needs a sign-off (git commit -s) — CI enforces DCO.

If this fixes a bug, a reproduction we can run is worth more than a description of the symptom.

Thanks for contributing, and sorry in advance for the wait.

@DeusData DeusData added bug Something isn't working parsing/quality Graph extraction bugs, false positives, missing edges ux/behavior Display bugs, docs, adoption UX priority/high Needs near-term maintainer attention; high-impact bug, regression, safety issue, or release blocker. labels Sep 19, 2026
@DeusData

Copy link
Copy Markdown
Owner

Thank you for carrying the stale-range work into this focused replacement. Current source still uses indexed source ranges, with freshness checks forming a separate contract. Review needs to retain the supplied boundary cases and check the Windows metadata and untracked-store behavior explicitly. The queue is busy, so it may take a little time to complete that review.

@DeusData

Copy link
Copy Markdown
Owner

Thank you — rejecting stale AST coordinates after the source has changed is exactly the kind of correctness fix this project cares about. Returning a snippet keyed to bytes that have moved is worse than returning nothing.

Two blockers, both small.

1. The memory-core linter (this is what lint / lint is failing on, not clang-format):

memory-core linter FAILED: raw allocator use grew.
  src/mcp/mcp.c: grew by 1 (814 -> 815); latest sites: 18147:free

One raw free at src/mcp/mcp.c:18147. Every allocation here routes through src/foundation/mem_core.h rather than the ~800 raw sites this codebase used to have, because the memory accounting, the budget and the spill machinery all depend on seeing each one — a raw free is invisible to them. The linter ratchets, so existing sites are tolerated and new ones are not.

Use cbm_free (and cbm_alloc / cbm_calloc / cbm_realloc / cbm_mem_strdup for the rest of the family). Keep the pairing consistent: memory that came from cbm_alloc must go to cbm_free, and memory from a raw malloc elsewhere must still go to raw free. If line 18147 releases something allocated by code you did not touch, tell me rather than guessing and we will sort it out.

make -f Makefile.cbm lint-ci reproduces it locally.

2. The dco check is red. Your commits have no Signed-off-by trailer — the message is a headline with an empty body, so there is nothing for the check to parse. (Not the blank-line trap, where the sign-off sits outside the trailer block and git's parser skips it while grep still sees it — here there is no trailer at all.)

git commit --amend -s          # single commit
git rebase --signoff origin/main   # several
git push --force-with-lease

#2241 and #2242 merged cleanly today with correct sign-offs, so the habit is already there — these branches just predate it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working parsing/quality Graph extraction bugs, false positives, missing edges priority/high Needs near-term maintainer attention; high-impact bug, regression, safety issue, or release blocker. ux/behavior Display bugs, docs, adoption UX

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bug: Stale AST line coordinates in get_code_snippet / search_code after file modification

2 participants